feat(agents): add the oracle browser researcher lane - #535
Conversation
Design a subscription-cookie research lane: Agent Pantry supplies the gemini.google.com cookies, the oracle CLI drives the browser session, and Brigade adds one CLI adapter. Also records the research-engine timeout gap found while planning: per-call timeouts are hardcoded at the call site and Caps silently drops unknown keys, so a slow backend needs a roster-driven min_timeout floor.
…lures The auth detail shared the provider-preflight branch, so every stale-cookie failure was reported as failure_kind=workspace-trust. Outcome capture and the model scorecard read that field, so a browser auth problem was being scored as a workspace trust refusal. Adds regression coverage through run_agent for both failure paths, a real-exec stub proving the argv reaches the process, and an engine-level test proving the timeout floor survives the planning call.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds an Oracle browser CLI adapter with hard read-only enforcement, model pinning, authentication diagnostics, and roster support. It also adds researcher timeout floors to ChangesOracle adapter and research integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant run_agent
participant Oracle
Caller->>run_agent: request Oracle completion
run_agent->>Oracle: execute --engine browser -p prompt
Oracle-->>run_agent: stdout or authentication failure
run_agent-->>Caller: markdown or classified failure
sequenceDiagram
participant DeepResearcher
participant CliBackend
participant _run_cli
DeepResearcher->>CliBackend: complete with requested timeout
CliBackend->>_run_cli: forward timeout floor
_run_cli-->>DeepResearcher: return research completion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/brigade/agents.py (2)
643-651: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"oracle" missing from the known-CLI error message.
_ADAPTERSnow includes"oracle"(line 308), but thisValueError's enumerated list of known CLIs doesn't mention it, so a typo'd oracle reference (e.g."orcale") won't surfaceoracleas a valid option.🩹 Proposed fix
raise ValueError( f"unknown agent cli: {cli_ref!r} " "(known: claude, codex, opencode, antigravity, pi, cursor, aider, goose, continue, " - "copilot, qwen, kimi, adal, openhands, grok, amp, crush, ollama:<model>, " + "copilot, qwen, kimi, adal, openhands, grok, amp, crush, oracle, ollama:<model>, " "codex-cloud:<env-id>)" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/brigade/agents.py` around lines 643 - 651, Update the known-CLI list in the ValueError raised by the adapter lookup around _ADAPTERS and builder so it includes “oracle,” matching the supported keys in _ADAPTERS. Keep the existing validation and error behavior unchanged.
1113-1124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated oracle-auth/provider-preflight classification logic.
The
oracle_auth = _oracle_auth_detail(...);provider_preflight = oracle_auth or _provider_preflight_detail(...);failure_kind = "browser-auth" if oracle_auth else "workspace-trust"sequence is repeated verbatim between the nonzero-exit branch and the empty-output branch. Extracting a small helper would keep the "auth wins over workspace-trust" precedence rule defined once.♻️ Proposed refactor
+def _classify_provider_preflight(cli_ref: str, stdout: str, stderr: str) -> tuple[str | None, str | None]: + """Return (detail, failure_kind) for a provider-preflight failure, or (None, None).""" + oracle_auth = _oracle_auth_detail(cli_ref, stdout, stderr) + detail = oracle_auth or _provider_preflight_detail(cli_ref, stdout, stderr) + if detail is None: + return None, None + return detail, "browser-auth" if oracle_auth else "workspace-trust"Then at each call site:
- oracle_auth = _oracle_auth_detail(cli_ref, safe_stdout, safe_stderr) - provider_preflight = oracle_auth or _provider_preflight_detail(cli_ref, safe_stdout, safe_stderr) + provider_preflight, preflight_kind = _classify_provider_preflight(cli_ref, safe_stdout, safe_stderr) if provider_preflight is not None: return AgentResult( ... - failure_kind="browser-auth" if oracle_auth else "workspace-trust", + failure_kind=preflight_kind,Also applies to: 1208-1213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/brigade/agents.py` around lines 1113 - 1124, Extract the repeated oracle-auth/provider-preflight classification from the nonzero-exit and empty-output branches into a small helper near the existing detail functions. Have the helper preserve auth precedence, returning the selected detail and corresponding failure kind (“browser-auth” when oracle auth exists, otherwise “workspace-trust”), then update both branches to use it for AgentResult construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_roster.py`:
- Around line 1410-1417: Rename test_load_accepts_oracle_researcher or retarget
its fixture and assertion so they consistently cover the intended seat. Since
VALID assigns agents.coder the “write code” role, either rename the test to
reflect the coder path or modify the fixture and assertion to target a
researcher agent.
---
Nitpick comments:
In `@src/brigade/agents.py`:
- Around line 643-651: Update the known-CLI list in the ValueError raised by the
adapter lookup around _ADAPTERS and builder so it includes “oracle,” matching
the supported keys in _ADAPTERS. Keep the existing validation and error behavior
unchanged.
- Around line 1113-1124: Extract the repeated oracle-auth/provider-preflight
classification from the nonzero-exit and empty-output branches into a small
helper near the existing detail functions. Have the helper preserve auth
precedence, returning the selected detail and corresponding failure kind
(“browser-auth” when oracle auth exists, otherwise “workspace-trust”), then
update both branches to use it for AgentResult construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d44d4f5d-f0bd-48a7-baf6-dc712edcb940
⛔ Files ignored due to path filters (2)
docs/phase-oracle-browser-researcher.mdis excluded by!docs/**,!**/docs/**docs/seat-catalog.mdis excluded by!docs/**,!**/docs/**
📒 Files selected for processing (6)
src/brigade/agents.pysrc/brigade/research/llm.pytests/test_agents_oracle.pytests/test_research_engine.pytests/test_research_llm.pytests/test_roster.py
| def test_load_accepts_oracle_researcher(tmp_path): | ||
| # Clears both gates: agent_adapters.is_known (from _ADAPTERS) and the | ||
| # limits.allow_models allowlist the VALID fixture declares. | ||
| text = VALID.replace('cli = "ollama:llama3.3"', 'cli = "oracle"').replace( | ||
| 'allow_models = ["codex", "ollama:*"]', 'allow_models = ["codex", "oracle"]' | ||
| ) | ||
| loaded = roster_mod.load_roster(_write(tmp_path, text)) | ||
| assert loaded.agents["coder"].cli == "oracle" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the role of the "coder" agent (and any "researcher" agent) in the VALID roster fixture.
rg -n -B2 -A15 '^VALID = ' tests/test_roster.py | head -80
rg -n 'role = "researcher"|role = "coder"' tests/test_roster.pyRepository: escoffier-labs/brigade
Length of output: 981
Rename or retarget this test. VALID assigns agents.coder the role "write code", so this case exercises the coder seat, not a researcher seat. If the intent is to cover the researcher path, change the fixture or assertion accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_roster.py` around lines 1410 - 1417, Rename
test_load_accepts_oracle_researcher or retarget its fixture and assertion so
they consistently cover the intended seat. Since VALID assigns agents.coder the
“write code” role, either rename the test to reflect the coder path or modify
the fixture and assertion to target a researcher agent.
|
Retargeted rather than renamed, in `a38f7c7`. The finding is correct: `VALID` gives `agents.coder` the role `"write code"`, so the test never built a researcher seat despite its name. Renaming it to `..._coder` would have made the name honest but left the researcher path uncovered at the roster layer. Instead the fixture now also swaps the role, and the test asserts through `find_role("researcher")` — which is the exact lookup `research/llm.py:resolve_backend` performs — so the seat this PR actually wires is the one under test: text = (
VALID.replace('cli = "ollama:llama3.3"', 'cli = "oracle"')
.replace('role = "write code"', 'role = "researcher"')
.replace('allow_models = ["codex", "ollama:*"]', 'allow_models = ["codex", "oracle"]')
)
loaded = roster_mod.load_roster(_write(tmp_path, text))
assert loaded.agents["coder"].cli == "oracle"
assert loaded.find_role("researcher").cli == "oracle"`"write code"` occurs exactly once in `VALID`, so the replace is unambiguous. `./scripts/verify` green: 4220 passed, 3 skipped. Note for anyone reading the diff: `docs/**` is path-excluded from this review, so `docs/phase-oracle-browser-researcher.md` was not looked at. It carries the design rationale, the recorded blockers, and the proof/unproven split. |
1 participant
What
Adds
oracleas a Brigade CLI adapter pinned to its browser engine, so the roster'sresearcherrole can run on a subscription cookie lane with no API key.steipete/oracle (MIT) drives a real
gemini.google.comsession using Chrome cookies. Agent Pantry already syncs that cookie jar between machines and warns before it expires, so the auth substrate already exists. This PR is the adapter that connects the two.oracle --engine browsersrc/brigade/agents.pysrc/brigade/research/llm.pyBackend selection needed no change:
research/llm.pyalready returns aCliBackendfor any researcher declaringcli, andengine.pyonly calls.complete().Brigade does not install oracle, bundle Node, or touch a browser. It shells out and reads text back, the same as every other adapter.
Why the timeout change rides along
research/engine.py:138asks fortimeout=30on its planning call andresearch/types.py:50defaults to 60, both hardcoded at the call site. A browser round trip will not meet 30 seconds, so the lane fails on its first request without a floor.Capshas no timeout field, andCaps.buildfilters overrides withhasattr, so a config-only attempt would look applied and silently do nothing.This is not a timeout widened to hide a symptom. It adds the missing mechanism for a backend to declare how slow it legitimately is, reusing the roster's existing per-agent
timeout_secondsthatresolve_backendwas discarding. It only raises, never lowers, and seats that declare notimeout_secondskeep today's behavior exactly.Happy to split this into its own PR if you would rather review it separately.
Where to look hardest
failure_kindis scored, so it has to be right. The first version of the cookie-expiry hint shared theprovider-preflightbranch and inheritedfailure_kind="workspace-trust", so every stale-cookie failure was being scored as a workspace trust refusal byoutcome captureandbrigade model scorecard. Now"browser-auth", fixed at both failure paths inrun_agent, with a codex regression guard proving the original kind still fires. Unit tests on the detail function alone had passed. Only a test throughrun_agentcaught it.Read-only is
hardby construction. Oracle has no filesystem write path, so it is the only adapter reportinghardwithout a sandbox or a prompt instruction. Registering it also makescli = "oracle"assignable to any seat rather than only the researcher. That is why the enforcement contract is tested rather than assumed.How to verify
4220 passed, 3 skipped, coverage 82.68% against the 78% floor.
Oracle-specific tests:
--heartbeat, never an API pathexecrun_agentreturns it verbatimrun_agentpathsfailure_kind="browser-auth"workspace-trustDeepResearcherasserts 300, not the engine's 30cli = "oracle"is_knownandlimits.allow_modelsWhat is not proven
oracleis not installed on the machine this was written on, so no real browser round trip ran. Two claims rest on reading oracle's source rather than executing it:src/cli/renderOutput.tsreturning unrendered markdown whenrichTtyis false, which it is under captured pipes, plus never passing--heartbeat. If a real run shows progress chrome, the fix is an extraction function following the_parse_grok_final_outputprecedent, not looservalidate_final_output.The full proof and blocker record is in
docs/phase-oracle-browser-researcher.md.Risk
Oracle's browser mode is labelled experimental by its author, and cookie automation of a consumer web session is grey against provider terms. The docs scope it to a single operator machine and explicitly not a fleet default. The
cli = "oracle"seam keeps the driver replaceable without touchingresearch/.@coderabbitai review
Summary by CodeRabbit